-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TeamCity: add Ephemeral-write-only subproject #12538
TeamCity: add Ephemeral-write-only subproject #12538
Conversation
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋🏻 Hey, here's some comments but I'm happy to approve this PR after they're resolved (either via discussion only or via code changes we identify to make).
mmv1/third_party/terraform/.teamcity/components/get_services.kt
Outdated
Show resolved
Hide resolved
mmv1/third_party/terraform/.teamcity/components/get_services.kt
Outdated
Show resolved
Hide resolved
// These are the packages that have resources that will use write-only attributes | ||
var ServicesListWriteOnlyGA = getServicesList(arrayOf("compute", "secretmanager", "sql", "bigquerydatatransfer"), "GA") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see that the new project will be limited to only the necessary services! 🙌🏻 Though the Compute build is one of our longest running builds... so running those tests in the feature branch testing project as well as the nightlies could cause some issues.
That's something to keep an eye on, at least. You could even add services to this list progressively over time, instead of adding them all now, to limit impact. Or you could change the trigger time (see other comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was also the idea of changing the test prefix within the write-only subproject to only run Tests with TestAccEphemeral_
though we would miss out on making sure all other tests aren't affected.
I could ensure that all other tests pass by manually triggering them to run the suite entirely, I'm leaning towards this in order to not have to make any TC updates in the future such as including it as part of the getServices
list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good to me, as long as you're aware there's a potential problem and you've got options about how to navigate it then I'm good 👍🏻
val trigger = NightlyTriggerConfiguration( | ||
branch = "refs/heads/$featureBranchEphemeralWriteOnly" // triggered builds must test the feature branch | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see other comment about conflicting with the nightly tests, particularly for Compute.
To mitigate this, you could update the trigger in the feature branch test project to run on a later schedule versus the nightlies. E.g. default time + 6 hours
83bae16
to
0ce1327
Compare
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
1 similar comment
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
0ce1327
to
519e6be
Compare
519e6be
to
18aa211
Compare
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
although this was approved, it could use a quick PR check since I introduced support for custom test prefix. Currently this will only run tests with the regex This can be ignored after seeing #12538 (comment) |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Note
This will be marked as Ready for Review once we have a 1.11 TF alpha version ready
Marking Ready for Review with the release of
v1.11.0-alpha20241211
This project is meant to be used for testing Write-Only attributes on resources that will support it in Terraform 1.11
It will do the following:
Here's a preview of the changes
How Triggers are set
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.